Preserve cached retry data when rebuilding Gantt chart#596
Conversation
When task instances are refreshed, rebuild_gantt() previously replaced the entire GanttData, wiping out previously fetched retry history. This caused a visible flicker as retries disappeared momentarily before being re-fetched from the API. Now, cached retry data is carried over into the new GanttData when it has more detailed history than the fresh data, and is only replaced when the new retry fetch completes. https://claude.ai/code/session_01RorM9BtWu7NUZB4yzCPFMQ
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughMakes GanttData::recompute_window public and updates TaskInstanceModel to track current DAG run, reset cached Gantt data on DAG-run change, and preserve cached retry tries when rebuilding Gantt data; state syncing now sets the DAG run id before loading task instances. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/model/taskinstances.rs`:
- Around line 84-92: The cache in gantt_data.task_tries is keyed only by TaskId
and can leak retry history across DAG runs; update the logic to clear or
reinitialize gantt_data (or at least gantt_data.task_tries) when dag_run_id
changes instead of carrying over cached_tries in rebuild_gantt; specifically
detect the dag_run_id switch in sync_panel or in the
WorkerMessage::UpdateTaskInstances handler and reset gantt_data.task_tries (or
the whole gantt_data) before calling rebuild_gantt/merging new_gantt so
cached_tries cannot be applied to a different run.
The gantt_data.task_tries cache is keyed by TaskId, which is reused across DAG runs of the same DAG. Without tracking which DAG run the cache belongs to, retry history from a previous run could leak into a newly selected run via rebuild_gantt()'s carry-over logic. Add a current_dag_run_id field to TaskInstanceModel and reset gantt_data when the DAG run changes in sync_panel. https://claude.ai/code/session_01RorM9BtWu7NUZB4yzCPFMQ
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/model/taskinstances.rs`:
- Around line 36-38: Cached gantt_data/task_tries currently only tracks
current_dag_run_id which allows collisions when different DAGs share the same
run id; change the cache key to include both DAG id and run id (e.g., add or
replace current_dag_run_id: Option<DagRunId> with a combined
current_dag_and_run: Option<(DagId, DagRunId)> or add a separate current_dag_id:
Option<DagId>) and update the invalidation logic that checks the run id (the
comparison around task_tries/gantt_data invalidation) to require both dag_id and
dag_run_id match before reusing cached data so caches are not shared across
different DAGs with the same run id.
DAG run IDs like "scheduled__2024-01-01T00:00:00" are commonly shared across different DAGs. The previous cache key (DagRunId only) meant navigating from DAG A's "run_1" to DAG B's "run_1" would skip the gantt_data reset since the run ID appeared unchanged. Replace current_dag_run_id with current_gantt_key: Option<(DagId, DagRunId)> so the cache is invalidated whenever either the DAG or the run changes. https://claude.ai/code/session_01RorM9BtWu7NUZB4yzCPFMQ
Summary
This change prevents the Gantt chart from flickering when retry data is being fetched by preserving previously cached try history during chart rebuilds.
Key Changes
rebuild_gantt()to preserve cached retry data from the previous Gantt state when fresh data is being loadedrecompute_window()public inGanttDatato allow recalculation after updating cached datarebuild_gantt()to build the new Gantt data separately before applying cache preservation logicImplementation Details
The rebuild process now:
GanttDatafrom current task instancesThis ensures smooth UX by maintaining detailed retry information while fresh data is being fetched from the server.
https://claude.ai/code/session_01RorM9BtWu7NUZB4yzCPFMQ
Summary by CodeRabbit
New Features
Bug Fixes